-
Notifications
You must be signed in to change notification settings - Fork 121
[Booking] Fix empty state after reload #16417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
7ed97aa to
1529bb5
Compare
|
|
b19aaa2 to
21b3d5e
Compare
itsmeichigo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works as described 👍 I left some questions below but pre-approving.
| func onRefreshSelfAction(reason: String? = nil) async { | ||
| await withCheckedContinuation { continuation in | ||
| paginationTracker.resync(reason: Self.refreshCacheReason) { | ||
| paginationTracker.resync(reason: reason ?? Self.refreshCacheReason) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about keeping the refresh cache as the default value for simplicity? And should we rename the method to reloadData for clarity?
| func onRefreshSelfAction(reason: String? = nil) async { | |
| await withCheckedContinuation { continuation in | |
| paginationTracker.resync(reason: Self.refreshCacheReason) { | |
| paginationTracker.resync(reason: reason ?? Self.refreshCacheReason) { | |
| func reloadData(reason: String = Self.refreshCacheReason) async { | |
| await withCheckedContinuation { continuation in | |
| paginationTracker.resync(reason: reason) { |
| todayListViewModel | ||
| return todayListViewModel | ||
| case .upcoming: | ||
| upcomingListViewModel | ||
| return upcomingListViewModel | ||
| case .all: | ||
| allListViewModel | ||
| return allListViewModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious: why is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed at all! I was just fiddling around with the tabs, and I guess I haven't fully reverted one of my changes. I'll remove the returns.
| /// Called when the user pulls down the list to refresh. | ||
| @MainActor | ||
| func onRefreshAction() async { | ||
| func onRefreshAction(reason: String? = nil) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change necessary? Unrelated but looks like the pull to refresh functionality doesn't work on the search view any more.

Closes WOOMOB-1795
Description
The root of the issue was that on pull-to-refresh, we cleared the bookings from cache and only loaded the currently displayed tab's bookings into the store. This PR fixes this by handling pull-to-refresh across all three tabs rather than just the viewed tab.
On pull-to-refresh we manually clear the cache and then load all three tabs' bookings. This way, there is no data loss.
More info here
Test Steps
Screenshots
RELEASE-NOTES.txtif necessary.